Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/cors #263

Merged
merged 9 commits into from
Jul 11, 2024
Merged

Feature/cors #263

merged 9 commits into from
Jul 11, 2024

Conversation

marest94
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@IgorBalog-Eng IgorBalog-Eng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to set following
#CORS configuration
#Allow specific origins
application.cors.allowed.origins=
#Allow specific HTTP methods
application.cors.allowed.methods=
#Allow specific headers
application.cors.allowed.headers=
also in GHA properties?

}

UrlBasedCorsConfigurationSource source = new UrlBasedCorsConfigurationSource();
source.registerCorsConfiguration("/**", configuration);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need cors for all ecc endpoints or just for API?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To get self-description, you're pointing to the root of ECC, so setting only API would cause CORS issues, and that's the reason why CORS is for all endpoints, but also as future proof if there will be any new modifications/features.

private RejectionMessageService rejectionMessageService;
private FileStreamingBean fileStreamingBean;
private ResponseMessageBufferClient responseMessageBufferClient;
private InputStreamSocketListenerClient inputStreamSocketListenerWebSocketClient;

@Autowired
private TLSProvider tlsProvider;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove annotation since it is set via constructor?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I forgot to delete it when I refactored it.

@marest94
Copy link
Collaborator Author

Do you need to set following #CORS configuration #Allow specific origins application.cors.allowed.origins= #Allow specific HTTP methods application.cors.allowed.methods= #Allow specific headers application.cors.allowed.headers= also in GHA properties?

In general, it's not mandatory, since there is a check, if it is not present, values would be set to "*" - so for everything.
But in order to be aligned with the rest of properties, it will be added.

@marest94 marest94 merged commit c8d00a2 into develop Jul 11, 2024
13 checks passed
@marest94 marest94 deleted the feature/cors branch July 11, 2024 14:07
marest94 added a commit that referenced this pull request Jul 11, 2024
* Update properties

* Upgrade GHA to use Node.js 20

* Improve handling attributes in firewall

* Modify SecurityConfig to use CORS config from property file

* Fix wss ssl

* Update pom.xml to use latest MMP

* Update CHANGELOG.md and README.md

* Address PR comments

* Update test to check PUT method, since OPTIONS is now allowed in
Firewall

Co-authored-by: Igor Balog Eng <[email protected]>
marest94 added a commit that referenced this pull request Jul 11, 2024
* Feature/cors (#263) (#264)

* Update properties

* Upgrade GHA to use Node.js 20

* Improve handling attributes in firewall

* Modify SecurityConfig to use CORS config from property file

* Fix wss ssl

* Update pom.xml to use latest MMP

* Update CHANGELOG.md and README.md

* Address PR comments

* Update test to check PUT method, since OPTIONS is now allowed in
Firewall

Co-authored-by: Igor Balog Eng <[email protected]>

* [maven-release-plugin] prepare release 1.14.9

* [maven-release-plugin] prepare for next development iteration

---------

Co-authored-by: Marko <[email protected]>
Co-authored-by: Igor Balog Eng <[email protected]>
Co-authored-by: GitHub Actions <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants